Skip to content

Conversation

@turt2live
Copy link
Member

@turt2live turt2live marked this pull request as ready for review June 9, 2025 21:38
@turt2live turt2live requested a review from a team as a code owner June 9, 2025 21:38
Copy link
Contributor

@H-Shay H-Shay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@H-Shay H-Shay merged commit 41a0ca2 into main Jun 12, 2025
5 checks passed
@H-Shay H-Shay deleted the travis/redact-on-ban branch June 12, 2025 23:24
Comment on lines +490 to 498
// Use MSC4293 and still fall back later
// See https://github.com/matrix-org/matrix-spec-proposals/pull/4293
await this.client.sendStateEvent(roomId, "m.room.member", member.userId, {
"membership": "ban",
"org.matrix.msc4293.redact_events": true,
});

// Fallback
this.redactUser(member.userId, roomId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a risk here if Mjolnir fails to redact events in the fallback. This can happen if there are more events in the timeline than the pagination limit, or if a call to /redact fails. Trying to workaround by using !mjolnir redact will no longer work in this room because Mjolnir will no longer receive unredacted events from /messages due to the behavior of MSC4293.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Receiving redacted events might be fine though? The logic could probably inspect the reason if it wanted too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, It could be fine, but only because mjolnir doesn't exclude the redacted events from being called with /redact again

mjolnir/src/utils.ts

Lines 102 to 121 in 41a0ca2

await getMessagesByUserIn(client, userIdOrGlob, targetRoomId, limit, async (eventsToRedact) => {
for (const targetEvent of eventsToRedact) {
await managementRoom.logMessage(
LogLevel.DEBUG,
"utils#redactUserMessagesIn",
`Redacting ${targetEvent["event_id"]} in ${targetRoomId}`,
targetRoomId,
);
if (!noop) {
await client.redactEvent(targetRoomId, targetEvent["event_id"]);
} else {
await managementRoom.logMessage(
LogLevel.WARN,
"utils#redactUserMessagesIn",
`Tried to redact ${targetEvent["event_id"]} in ${targetRoomId} but Mjolnir is running in no-op mode`,
targetRoomId,
);
}
}
});

// See https://github.com/matrix-org/matrix-spec-proposals/pull/4293
await this.client.sendStateEvent(roomId, "m.room.member", member.userId, {
"membership": "ban",
"org.matrix.msc4293.redact_events": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was reason intentionally omitted here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants